-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce transcription protocol #194
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
thanks for fixing the CI too
@@ -117,6 +131,42 @@ async def publish_data( | |||
if cb.publish_data.error: | |||
raise PublishDataError(cb.publish_data.error) | |||
|
|||
async def publish_transcription( | |||
self, | |||
participant_identity: str, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would be more comfortable creating a Transcription structure here.
On the agents API, we will need to return a Transcription, and I don't think this should be defined inside agents.
publish_transcription accepting a "Transcription" seems OK to me
Also this will be needed for the "on_transcription_received" event
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also wondering if start_time and end_time should be float on the Python side.
To respect the time.time() format.
Maybe we could automatically do the conversion?
@@ -0,0 +1,10 @@ | |||
from dataclasses import dataclass | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add it here? (Transcription struct)
@theomonnom re: float. I think we should keep it int because the clock being used will be the frame clock. Although I'm wondering if we should specify units in the transcription protocol now: |
let's mb wait for a release tho? |
No description provided.